Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove geosphere dependency #236

Merged
merged 10 commits into from
Aug 16, 2024

Conversation

mhpob
Copy link
Contributor

@mhpob mhpob commented May 24, 2024

Current R CMD check failure is due to a floating point issue on macOS which will be fixed with acceptance of #228

This PR removes dependency on geosphere (See #235).

May introduce a breaking change in point_offset by using the Haversine (great circle) method as originally described in the documentation as opposed to a geodesic method (ellipsoid) as used by geosphere. Creates differences on the order of <5 meters in distances less than a few dozen kilometers, but can increase to nearly 2 kilometers on the scale of the entire Great Lakes area.

Adds initial tests for point_offset and interpolate_path as none currently exist.

@chrisholbrook
Copy link
Collaborator

Given that this function was really just a wrapper for geoshere::destPoint() that allows input in the form of compass point abbreviations (e.g., "NNE", "S"), I support this change and also suggest we deprecate this function in favor of a new function that simply converts the compass abbreviations to (nominal) numerical equivalents. Then suggest using geosphere::distPoint() with those as input.

point_offset() is not used anywhere within this package, though I am aware of at least two people who use it to derive approximate location of snag line anchors (to facilitate receiver recovery).

@chrisholbrook chrisholbrook merged commit a20ca37 into ocean-tracking-network:dev Aug 16, 2024
@mhpob mhpob deleted the dev-rm-geosphere branch August 25, 2024 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants